Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use uv-pre-commit to validate lockfile #6699

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Use uv-pre-commit to validate lockfile #6699

wants to merge 2 commits into from
+9 −18

Conversation

danielhollas
Copy link
Collaborator

The previous uv hook that checked whether uv lockfile is up-to-date required the developer to have uv installed. Using the official uv-pre-commit hook, this is no longer the case. The hook also updates the lock automatically, instead of just checking its status.

Here's the output that I got when I bumped the mypy package in pyproject.toml

❯ pre-commit run uv-lock --all
uv-lock..................................................................Failed
- hook id: uv-lock
- files were modified by this hook

warning: `VIRTUAL_ENV=/home/hollas/.cache/pre-commit/repovyb9qrvk/py_env-python3.12` does not match the project environment path `.venv` and will be ignored
Resolved 263 packages in 2.50s
Updated mypy v1.13.0 -> v1.14.1

(the warning is a bit unfortunate interaction between uv and pre-commit, I opened astral-sh/uv-pre-commit#36 and asked if it could be hidden so that it is not confusing for devs)

danielhollas
danielhollas commented Jan 10, 2025
View reviewed changes
.github/workflows/test-install.yml
@@ -45,9 +45,6 @@ jobs:
- name: Install utils/ dependencies
run: uv pip install --system -r utils/requirements.txt

- name: Validate uv lockfile
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to have this check only in one place.

@codecov Codecov
Copy link

codecov bot commented Jan 10, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.00%. Comparing base (c88fc05) to head (7d77899).

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #6699   +/-   ##
=======================================
  Coverage   78.00%   78.00%           
=======================================
  Files         563      563           
  Lines       41766    41766           
=======================================
  Hits        32574    32574           
  Misses       9192     9192

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danielhollas
Copy link
Collaborator Author

danielhollas commented Jan 10, 2025 •
edited
Loading

Hmm, this might be a problem, looks like pre-commit.ci blocks network requests. I think I've read about this in uv issue tracked

Using CPython 3.10.12 interpreter at: /usr/bin/python3
error: Failed to generate package metadata for `aiida-core==2.6.2.post0 @ editable+.`
  Caused by: Failed to resolve requirements from `build-system.requires`
  Caused by: No solution found when resolving: `flit-core>=3.4, <4`
  Caused by: Failed to fetch: `https://pypi.org/simple/flit-core/`
  Caused by: Could not connect, are you offline?
  Caused by: Request failed after 3 retries
  Caused by: error sending request for url (https://pypi.org/simple/flit-core/)
  Caused by: client error (Connect)
  Caused by: dns error: failed to lookup address information: Temporary failure in name resolution
  Caused by: failed to lookup address information: Temporary failure in name resolution

EDIT: Indeed, this is a known issue when using dynamic metadata (such as version in our case) in `pyproject.toml: astral-sh/uv-pre-commit#35

@danielhollas danielhollas marked this pull request as draft January 10, 2025 17:45
@danielhollas
Copy link
Collaborator Author

This might get resolved in next uv version: astral-sh/uv#10622

danielhollas
danielhollas commented Jan 15, 2025
View reviewed changes
.pre-commit-config.yaml Outdated Show resolved Hide resolved
@unkcpz unkcpz mentioned this pull request Jan 16, 2025
Open
@danielhollas danielhollas force-pushed the pre-commit-uv branch 2 times, most recently from c7189a8 to 3766c25 Compare January 16, 2025 12:09
@danielhollas danielhollas mentioned this pull request Jan 16, 2025
Merged
@danielhollas
Use uv-pre-commit to validate lockfile
ab3d92f 
There are two big advantages here:
1. Devs to have to have uv installed globally
2. We control uv version automatically.
@danielhollas
Bump setup-uv action to 5.2.1
7d77899
@danielhollas
Copy link
Collaborator Author

I notified the uv maintainers about the issue that blocks this PR and they already replied and it looks like it might not be difficult to fix. Other people are hitting the same issue as well so let wait a bit for the next release. astral-sh/uv#10689

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@danielhollas